Reminder for myself to rebase on https://github.com/Auterion/PX4_firmware_private/pull/661 like discussed. EDIT: ✔️

@dagar

• I rebased this pr on new manual_control_switches msg (split out of manual_control_setpoint) #16270 like I wrote here: new manual_control_switches msg (split out of manual_control_setpoint) #16270 (comment)
• I followed the flight review comments in Scale manual control setpoint up/down stick -1 to 1 #15949 (comment) but both prs need flight review adaptation
• I brought back QGC not complying with https://mavlink.io/en/messages/common.html#MANUAL_CONTROL compatibility because it's for another pr to sync this change see Scale manual control setpoint up/down stick -1 to 1 #15949 (comment)

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

I think for nicer usage in all the different modules it would be helpful to extend the Sticks class to provide the common scaling and gestures (is stick below minimum) from one single place.

I agree with you on this point. Seems a bit dangerous (error prone) that we are re-interpreting these manual controls everywhere they are used.

I rebased this pr on main and after investigating carefully I suggest using roll, pitch, yaw, and throttle for the naming of the axes because:

• It used to be that originally
  • in PX4: https://github.com/PX4/PX4-Autopilot/pull/922/files#diff-46577665f843b3c1f8f67ac8a331ccb48274ba41fc7c5b1ece223fc7ffe21424L68-L71
  • in MAVLink: https://github.com/mavlink/mavlink/pull/64/files#diff-9ef151b30f09d6d4f130434d941db479c3411ec15eca23580c5587b2c3b59ecdL1354-L1357
• Scanning through other drone-related projects and APIs the only convention I found is that one.
• I was looking for that when I joined PX4 and thought I'm the only one.
• @bkueng chose to use this as the most clear way to assign RC sticks to direct output passthrough
  

  PX4-Autopilot/src/lib/mixer_module/output_functions.yaml

  Lines 24 to 27 in 55563eb

  	RC_Roll: 402
  	RC_Pitch: 403
  	RC_Throttle: 404
  	RC_Yaw: 405

• @junwoo091400 when he started immediately asked where these axes are and then added helpers for his use:
  

  PX4-Autopilot/src/modules/flight_mode_manager/tasks/Utility/Sticks.hpp

  Lines 66 to 69 in 55563eb

  	float getPitch() const { return _positions(0); }
  	float getRoll() const { return _positions(1); }
  	float getThrottleZeroCentered() const { return -_positions(2); } // Convert Z-axis(down) command to Up-axis frame
  	float getYaw() const { return _positions(3); }

• Half of the usage of the stick assign to variables like roll, pitch, yaw, thrust
• Some usage has comments to remind the reader which one is which:
  

  PX4-Autopilot/src/lib/mixer_module/functions.cpp

  Lines 112 to 115 in 55563eb

  	_data[0] = manual_control_setpoint.y; // roll
  	_data[1] = manual_control_setpoint.x; // pitch
  	_data[2] = manual_control_setpoint.z * 2.f - 1.f; // throttle
  	_data[3] = manual_control_setpoint.r; // yaw

  
  

  PX4-Autopilot/src/modules/flight_mode_manager/tasks/Utility/Sticks.cpp

  Lines 54 to 58 in 55563eb

  	_positions(0) = manual_control_setpoint.x; // NED x, pitch [-1,1]
  	_positions(1) = manual_control_setpoint.y; // NED y, roll [-1,1]
  	_positions(2) = -(math::constrain(manual_control_setpoint.z, 0.0f,
  	1.0f) - 0.5f) * 2.f; // NED z, thrust resacaled from [0,1] to [-1,1]
  	_positions(3) = manual_control_setpoint.r; // yaw [-1,1]

• RC mapping parameters are called this way
  

  PX4-Autopilot/src/modules/rc_update/params.c

  Line 1168 in 55563eb

  PARAM_DEFINE_INT32(RC_MAP_ROLL, 0);

  
  

  PX4-Autopilot/src/modules/rc_update/params.c

  Line 1200 in 55563eb

  PARAM_DEFINE_INT32(RC_MAP_PITCH, 0);

  
  

  PX4-Autopilot/src/modules/rc_update/params.c

  Line 1299 in 55563eb

  PARAM_DEFINE_INT32(RC_MAP_YAW, 0);

  
  

  PX4-Autopilot/src/modules/rc_update/params.c

  Line 1267 in 55563eb

  PARAM_DEFINE_INT32(RC_MAP_THROTTLE, 0);

• When watching Youtube videos on how to operate vehicles using a remote the sticks are refered as roll, pitch, yaw, throttle stick
• When asking multiple people about how to refer to the axes the fastest and most consistent answer was roll, pitch, yaw, throttle

The only two disadvantages I see are:

• For the pitch stick to be consistent with positive values are stick up right it maps to a negative pitch euler angle which is documented and in my eyes acceptable but possibly the main discussion point. I'd suggest to not switch the direction because:
  • Up right stick being positive makes mapping a lot easier
  • Switching throttle and pitch e.g. for RC mode 1 would then also involve switching sign and is more error-prone
  • Staying consistent with the existing direction (also in MAVLink)
• Stick input usages that completely differ from the conventional way of input might get shuffled when remapping the assignments e.g. for RC Mode 1,2,3,4 which was already the case before and I don't see an easy fix without undue complexity.

For the revamping of the Stick class to adapt and be reusable everywhere I suggest to do a follow-up pull request since this one is about the uORB interface.

@junwoo091400 Thanks a lot for your review, additional thoughts and suggestions. Your feedback is highly appreciated just slightly on the verbose side 😇

QGC is fetching these values from the RC_CHANNELS message
doesn't depend on changing the definition of MANUAL_CONTROL

I know. The big problem is solving backward compatibility on all sides e.g. QGC, MAVLink to an ambiguous definition that was misused in various out-of-range or other "fits the case" solutions. I explicitly wanted this pr to only solve the PX4 autopilot side and care about MAVLink and QGC in the next step.

I had to rebase on #20647 and #20537 nothing complicated.

After the feedback from yesterday's dev call I found a good solution for the backward compatibility: I do the same as before but not adjusting the throttle trim parameter. That solves the problem of having an adapted parameter when downgrading but also makes sure that it all works like expected with the default calibration everyone gets through QGC. So the door is open for a new calibration and phasing out the old one.

I tested this again on hardware just to be sure. Also note that even though FMUv5x overflows this pr saves ~230bytes but main is overflowing more than that.

In my eyes this is now good to go 🎉

I found a good solution for the backward compatibility: I do the same as before but not adjusting the throttle trim parameter. That solves the problem of having an adapted parameter when downgrading but also makes sure that it all works like expected with the default calibration everyone gets through QGC.

Ah that's such a good idea! (Took me 10 seconds to understand though haha) Changing the run-time applied trim value, and not modifying the parameter value itself is an elegant solution 👍

I guess we should get the QGC PR in quickly! If you have a QGC dev environment, could you test on a real vehicle with real RC tx/rx whether the QGC PR: mavlink/qgroundcontrol#10497 works as expected (Set Trim to mid-value, not min)?

@junwoo091400 Thanks for your review!

I guess we should get the QGC PR in quickly!

I think we shouldn't over-hurry but rather avoid mistakes that could lead to user problems.

If you have a QGC dev environment

I currently don't sorry. But as I wrote in mavlink/qgroundcontrol#10497 (comment) we need to be careful. I'd guess with that pr the trim value doesn't necessarily get set to the middle but to where the throttle stick is at the beginning of the calibration which would be not deterministic and often at the bottom. Also think about the case where someone uses the latest QGC including that change but PX4 1.13 or main from yesterday. It would result in a configuration making half of the stick not work anymore for no reason. Let's go through it cleanly on the QGC side.

Yayyyy thanks for this @MaEtUgR !!

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/driving-backwards-with-a-boat-rover-in-manual-mode/25714/6